Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Restore the help modal in the classic block #11856

Merged
merged 1 commit into from Nov 20, 2018

Conversation

azaozz
Copy link
Contributor

@azaozz azaozz commented Nov 14, 2018

It has some duplicates with the main help modal, but many things are specific for the classic block. Also, its content can be "tweaked" specifically for the classic block in the wordpress MCE plugin (in core).

#9748

It has some duplicates with the main help modal, but many things are specific for the classic block. Also, can be "tweaked" specificlly for the classic block in the `wordpress` MCE plugin (in core).
@azaozz azaozz added the [Block] Classic Affects the Classic Editor Block label Nov 14, 2018
@azaozz azaozz added this to the WordPress 5.0 milestone Nov 14, 2018
@talldan
Copy link
Contributor

talldan commented Nov 15, 2018

Hey @azaozz - The main issue here is that you end up with both modals open at the same time when using the shortcut key from within the classic block:
screen shot 2018-11-15 at 12 03 41 pm

Not sure if there's a way to stop the gutenberg help opening.

I'm also wondering if it might be a bit confusing for users if there are inconsistent modals. If the title of the classic editor modal could be changed to something like 'Classic Block shortcuts' that might help.

@azaozz
Copy link
Contributor Author

azaozz commented Nov 15, 2018

Ah, I see. We should be able to fix this by "capturing" the key event in the wordpress plugin (stopPropagation, etc.). Lets see how we can do that in TinyMCE :)

Actually it may be good to do that for all TinyMCE shortcuts so they don't trigger something "upstream" when an MCE instance is focused.

If the title of the classic editor modal could be changed...

Yep, shouldn't be hard to do that, just need a "sure" way to detect when we are in the classic block instance.

@azaozz
Copy link
Contributor Author

azaozz commented Nov 15, 2018

Hmm, thinking more about this, it would be better to have just one "help modal". How about adding another section to the main Gutenberg modal for the Classic Block? Then can list just the specific shortcuts there, not repeat most of them :)

@talldan
Copy link
Contributor

talldan commented Nov 15, 2018

The current Gutenberg modal is getting pretty big, so it might need some thought on how to keep it easily navigable if we add more sections to it.

@azaozz
Copy link
Contributor Author

azaozz commented Nov 15, 2018

True, it's quite long even now :) Would need to look at making it two columns perhaps? Thinking something like that would probably be good?

help-modal-2-col

@mtias
Copy link
Member

mtias commented Nov 15, 2018

Closes #9748

@karmatosed
Copy link
Member

I don't think we should add more sections to the existing modal. It is ok to have 2 modals but it's not ok to show them both together. Classic is very specific and unless mistaken as to what is being suggested, it would be weird to see that information in the non-classic modal.

@ellatrix
Copy link
Member

We could override the shortcut if the classic block is selected?

@azaozz
Copy link
Contributor Author

azaozz commented Nov 16, 2018

@karmatosed Sure. Still thinking we could try making the "global" help popup responsive, i.e. switch to two columns when the viewport is wide enough, perhaps in v.2?

There may also be other blocks that have specific shortcuts. Thinking we may need an "uniform" way to display them? One thing I tried (just visually) was to move the "button" that opens the global popup to the "More options" menu on the block toolbar. May be nice to have it there, and we can tweak the content according to block type :)

@iseulde yeah, will need to override the shortcut in the Classic Block instance. Was wondering if we need to override all TinyMCE shortcuts so they don't trigger something else "upstream" when an instance is focused. Will also change the title to "Classic Block Keyboard Shortcuts" as discussed above.

@azaozz
Copy link
Contributor Author

azaozz commented Nov 16, 2018

will need to override the shortcut in the Classic Block instance

That means part of the fix for this will be in the wordpress TinyMCE plugin in core. Created https://core.trac.wordpress.org/ticket/45365 for it.

Copy link
Contributor

@youknowriad youknowriad left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's improve on that on a separate PR

@youknowriad youknowriad merged commit d5cf1d7 into master Nov 20, 2018
@youknowriad youknowriad deleted the fix/restore-classic-block-help-modal branch November 20, 2018 13:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Classic Affects the Classic Editor Block
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants